chat: enforce required tagged tool args in any order via permute() - #26472
Open
i386 wants to merge 1 commit into
Open
chat: enforce required tagged tool args in any order via permute()#26472i386 wants to merge 1 commit into
i386 wants to merge 1 commit into
Conversation
Author
|
@pwilkin could I please get your eyes on this one? |
Contributor
|
If this is targeting the Qwen models, those have moved to a specialized parser. Regardless, the changes here prevent the grammar sampler from enforcing required arguments. In its place, an error is returned if the model duplicates or omits a required property. That is not a solution, in my opinion. If anything, the request should succeed and allow the client to send a descriptive error message so the model can self-correct. In my experiments, Qwen follows the order in If the intent is to target other models, then a better solution is to leverage the recent |
i386
force-pushed
the
agent/llama-tagged-tool-argument-order
branch
from
August 29, 2026 21:10
cdf4755 to
46fec96
Compare
i386
changed the title
i386
added a commit
to Mesh-LLM/mesh-llm
that referenced
this pull request
Aug 29, 2026
Rework the tagged tool-call argument grammar in patch 0010 per upstream review of ggml-org/llama.cpp#26472: required arguments are now matched through permute(), which accepts any ordering while requiring each argument exactly once, so the grammar structurally rejects omitted and duplicated required arguments instead of accepting them and throwing a bespoke mapper-side error at parse time. Optional arguments keep a zero_or_more choice and may interleave anywhere among required ones (each permuted required slot is optional_run + required + space, plus a trailing optional_run, so optional args emitted between two required args still parse). Delete the mapper-side TOOL_REQUIRED_ARG_PREFIX tagging, the required_args/seen_args bookkeeping, and the Missing/Duplicate tool argument throws those supported. Update the two throw-asserting tests to assert grammar-level rejection instead. Mirrors the construction the generic JSON tool path already uses (common/chat.cpp), including the COMMON_CHAT_MAX_PERMUTE=6 fallback-to-sequence policy. Re-verified on the prepared tree: prepare-llama.sh pinned re-applies all patches cleanly; test-chat-auto-parser (533), test-chat-peg-parser (210) green; focused driver cases (interleaved, reversed, missing, duplicate-required, duplicate-optional) all correct. Signed-off-by: James Dumay <jameswdumay@gmail.com> Co-authored-by: Son Of Dario <f551f0290dbf6b68fe126a2679e2aec49f059b6d0d295923fe300fdd71c8ffb6@meshllm.communities.buzz.xyz> Co-authored-by: jy <6b90287aa37add903ef8b4200477ecbfa85ac0da1a84cf1ff51a7207d0220740@meshllm.communities.buzz.xyz>
i386
added a commit
to Mesh-LLM/mesh-llm
that referenced
this pull request
Aug 29, 2026
Rework the tagged tool-call argument grammar in patch 0010 per upstream review of ggml-org/llama.cpp#26472: required arguments are now matched through permute(), which accepts any ordering while requiring each argument exactly once, so the grammar structurally rejects omitted and duplicated required arguments instead of accepting them and throwing a bespoke mapper-side error at parse time. Optional arguments keep a zero_or_more choice and may interleave anywhere among required ones (each permuted required slot is optional_run + required + space, plus a trailing optional_run, so optional args emitted between two required args still parse). Delete the mapper-side TOOL_REQUIRED_ARG_PREFIX tagging, the required_args/seen_args bookkeeping, and the Missing/Duplicate tool argument throws those supported. Update the two throw-asserting tests to assert grammar-level rejection instead. Mirrors the construction the generic JSON tool path already uses (common/chat.cpp), including the COMMON_CHAT_MAX_PERMUTE=6 fallback-to-sequence policy. Re-verified on the prepared tree: prepare-llama.sh pinned re-applies all patches cleanly; test-chat-auto-parser (533), test-chat-peg-parser (210) green; focused driver cases (interleaved, reversed, missing, duplicate-required, duplicate-optional) all correct. Signed-off-by: James Dumay <jameswdumay@gmail.com> Co-authored-by: Son Of Dario <f551f0290dbf6b68fe126a2679e2aec49f059b6d0d295923fe300fdd71c8ffb6@meshllm.communities.buzz.xyz> Co-authored-by: jy <6b90287aa37add903ef8b4200477ecbfa85ac0da1a84cf1ff51a7207d0220740@meshllm.communities.buzz.xyz>
Author
|
@aldehir that makes a lot of sense and thank you for your gentle feedback here :) I've updated using |
Contributor
I don't want to support this, because it further complicates the generated grammar. We have to be mindful of how many active stacks may exist at any point in time in the grammar sampler. https://developers.openai.com/api/docs/guides/structured-outputs gives some insight in how OpenAI is handling this:
Right now For now, we should keep the ordering of |
i386
added a commit
to Mesh-LLM/mesh-llm
that referenced
this pull request
Aug 29, 2026
Port the ggml-org/llama.cpp#26472 reviewer-preferred design onto main's patch queue (the tagged tool-arg rework previously staged on the jd/apple-core-ai stack as 0010): required arguments in the tagged tool-call grammar are matched through permute(), which accepts any ordering while requiring each argument exactly once, so the grammar structurally rejects omitted and duplicated required arguments instead of accepting them and throwing a bespoke mapper-side error at parse time. Optional arguments keep a zero_or_more choice and may interleave anywhere among required ones (each permuted required slot is optional_run + required + space, plus a trailing optional_run). Deletes the TOOL_REQUIRED_ARG_PREFIX tagging, the required_args / seen_args bookkeeping, and the Missing/Duplicate tool argument throws. The two throw-asserting tests now assert grammar-level rejection ("does not match the expected format"). Mirrors the generic JSON tool path construction (common/chat.cpp), including the COMMON_CHAT_MAX_PERMUTE = 6 fallback-to-sequence policy. Verified against main's queue: prepare-llama.sh pinned applies all 32 patches clean onto pin cc83d7b4; test-chat-auto-parser and test-chat-peg-parser suites green on the prepared tree; the new interleaved/reversed/omitted/duplicated tagged-arg cases pass under test-chat --template GLM-4.7-Flash. Full test-chat is blocked by a pre-existing, unrelated fixture crash at test_msgs_oaicompat_json_conversion (reproduced identically on a pristine-main prepared tree with this change absent). Signed-off-by: James Dumay <jameswdumay@gmail.com> Co-authored-by: Son Of Dario <f551f0290dbf6b68fe126a2679e2aec49f059b6d0d295923fe300fdd71c8ffb6@meshllm.communities.buzz.xyz> Co-authored-by: jy <6b90287aa37add903ef8b4200477ecbfa85ac0da1a84cf1ff51a7207d0220740@meshllm.communities.buzz.xyz>
i386
force-pushed
the
agent/llama-tagged-tool-argument-order
branch
from
August 30, 2026 00:35
46fec96 to
a53deca
Compare
Object member order is semantically irrelevant, so tool calls in the generic tagged format (arg_key/arg_value tags) may emit required arguments in any order. Required arguments are now matched through permute(), which generates every ordering while requiring each argument exactly once -- the same approach the generic JSON tool path already uses (common/chat.cpp). The grammar therefore structurally rejects a call that omits or duplicates a required argument, instead of accepting it and throwing a bespoke mapper-side error at parse time. This keeps malformed calls out of the server and hands the client a descriptive "does not match the expected format" error. Optional arguments follow the required ones as a trailing run, in any order, each at most once -- the ordering of required + optional is kept, matching the JSON tool path. Above COMMON_CHAT_MAX_PERMUTE (6) required args, permute() falls back to a fixed sequence -- the same policy as the JSON tool path. Tests cover reversed required order, an optional argument in the trailing run, and grammar-level rejection of omitted and duplicated required arguments and of optional-before-required ordering.
i386
force-pushed
the
agent/llama-tagged-tool-argument-order
branch
from
August 30, 2026 00:35
a53deca to
264f66c
Compare
i386
changed the title
Author
|
@aldehir fair call - have updated to match what you've asked for. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Fixes the generic tagged tool-call output parser (
arg_key/arg_valuetags, e.g. the GLM-4.7-Flash template): it required allrequiredarguments to be emitted in schema definition order, before any optional ones, and rejected valid calls where the model reordered the required arguments. JSON object member order is semantically irrelevant, so required arguments must be accepted in any order.This does not touch the Qwen path — Qwen tool calls go through their own specialized parser (thanks @aldehir).
Approach
Required arguments are matched through
permute()(common/chat-peg-parser.cpp), which generates every ordering while requiring each argument exactly once — the same construction the generic JSON tool path already uses (common/chat.cpp). The grammar therefore structurally rejects a call that omits or duplicates a required argument; there is no relaxed grammar plus post-hoc mapper-side validation and no bespoke runtime error. A malformed call simply does not match the grammar, which keeps the grammar sampler enforcing the schema during constrained decoding.Optional arguments follow the required ones as a trailing run, in any order, each at most once — the ordering of required + optional is kept, matching the JSON tool path.
Caveat: above
COMMON_CHAT_MAX_PERMUTE(6) required arguments,permute()falls back to a fixed sequence — the same policy the JSON tool path already applies.Tests
test-chat,test-chat-peg-parserandtest-chat-auto-parsersuites passRequirements